feat: tag events handlers partial recovery#792
Conversation
|
@greptileai review |
Greptile SummaryThis PR refactors the tag event handlers to support partial recovery by adopting a graph-last deletion order, a
Confidence Score: 3/5Safe to merge for the DEL-side improvements; the PUT Updated retry path leaves a known correctness gap in counter/score recovery that should be addressed before this is relied upon in production. DEL handler changes are well-reasoned and tested. The PUT Updated branch is intentionally partial but could silently leave Redis counts wrong in a graph-wrote/Redis-never-ran failure scenario, which may be hard to detect without monitoring. nexus-watcher/src/events/handlers/tag.rs — Updated branches in put_sync_post and put_sync_user
|
| Filename | Overview |
|---|---|
| nexus-watcher/src/events/handlers/tag.rs | Core handler refactored for partial recovery: DEL now reads graph first and guards decrements via check_set_member; PUT retries idempotent Redis ops on Updated. Missing non-idempotent counter/score recovery in the Updated branch is a correctness concern. |
| nexus-common/src/models/tag/traits/collection.rs | Trait extended with get_target_from_graph and simplified del_from_graph (now uses exec_single_row for fire-and-forget delete). Logic is clean and idempotent-by-default. |
| nexus-common/src/db/graph/queries/get.rs | New get_tag_target query correctly separates the read from the delete to enable graph-last ordering. Cypher logic mirrors the removed portion of delete_tag. |
| nexus-common/src/db/graph/queries/del.rs | delete_tag simplified to a fire-and-forget DELETE with no RETURN clause; consistent with the new exec_single_row call site. |
| nexus-watcher/tests/event_processor/tags/del_idempotent.rs | Two new tests covering retry and replay paths; retry test verifies no double-decrement, replay test verifies idempotent Ok but lacks counter stability assertions. |
| nexus-watcher/tests/event_processor/tags/retry_post_tag.rs | Removed stale assertions that expected a SkipIndexing retry event after del; consistent with the new idempotent no-op behavior. |
| nexus-watcher/tests/event_processor/tags/retry_user_tag.rs | Same cleanup as retry_post_tag.rs — SkipIndexing assertions removed, aligning with idempotent no-op del behavior. |
| nexus-watcher/tests/event_processor/tags/mod.rs | Registers the new del_idempotent test module. No issues. |
Sequence Diagram
sequenceDiagram
participant H as tag::del handler
participant G as Neo4j Graph
participant R as Redis
Note over H: DEL flow (graph-last)
H->>G: get_tag_target(user_id, tag_id)
G-->>H: Some(tagged_id, post_id, author_id, label) OR None
alt Edge found
H->>R: check_set_member(tagger_in_index?)
R-->>H: bool
alt tagger_in_index == true
H->>R: decrement counters + scores
H->>R: send untag notification
end
H->>R: SREM tagger (idempotent)
H->>R: del_from_index / PostsByTagSearch cleanup (idempotent)
H->>G: del_from_graph (graph-last)
else Edge already gone (idempotent replay)
H-->>H: return Ok(()) silently
end
Note over H: PUT flow (Updated = prior attempt)
H->>G: put_to_graph(...)
G-->>H: Updated (edge existed)
H->>R: add_tagger_to_index (idempotent set-add)
H->>R: PostsByTagSearch::put_to_index (idempotent)
H->>R: TagSearch::put_to_index (idempotent)
Note over H,R: CounterIncrements / ScoreUpdates NOT retried
Reviews (1): Last reviewed commit: "review" | Re-trigger Greptile
ok300
left a comment
There was a problem hiding this comment.
A few NITs, but otherwise LGTM.
8c553b5 to
13f24c9
Compare
13f24c9 to
fbab9b4
Compare
|
|
||
| /// Reads tag target details from the graph without deleting the TAGGED edge. | ||
| /// Same return shape as `del_from_graph`. | ||
| async fn get_target_from_graph( |
There was a problem hiding this comment.
Unused dead code? get_target_from_graph was added, but is never used.
Prepares tag handlers for partial recovery.